Skip to content
This repository has been archived by the owner on Apr 20, 2023. It is now read-only.

[WIP] Reduce test target complexity [and running time] #5403

Conversation

TheRealPiotrP
Copy link

I was inspired by @blackdwarf's work to reduce the many targets of 'test restore' into a single sln and single restore invocation. Greatly reduced concept count and a slight perf improvement.

This change eliminates a bunch of duplicate test orchestration code, resulting in modest perf gains and lots of simplification. Included:

  • Tests are no longer built before execution. This saves ~7s per test project that is required for dotnet test to prove that the test library is up-to-date
  • CLI build infra no longer attempts to perform incrementality checks on behalf of dotnet build. This was necessary when we were wrapping project.json but is no longer needed.
  • Test Assets are also no longer pre-built, saving many cycles
  • No more incrementality checks from CLI build system for Test Assets
  • Test targets are significantly reduced in complexity [though still complex]

The PR is a WiP. I expect issues due to / vs \. I'm also not able to test locally due to pending reboot, so will see what CI has to say.

/cc @livarcocc

@TheRealPiotrP TheRealPiotrP force-pushed the piotrpMSFT/infra/reducetesttargetcomplexity branch from 88c3d44 to b56c0e0 Compare January 20, 2017 09:47
Piotr Puszkiewicz added 3 commits January 21, 2017 02:03
Remove deprecated tests
Make Microsoft.DotNet.Tools.Tests.Utilities portable-only
Remove MSI tests from the solution as they are the only  tests that currently require dekstop.
@TheRealPiotrP TheRealPiotrP force-pushed the piotrpMSFT/infra/reducetesttargetcomplexity branch from 7ef8cb8 to 4f466fb Compare January 21, 2017 10:40
@TheRealPiotrP TheRealPiotrP force-pushed the piotrpMSFT/infra/reducetesttargetcomplexity branch from 4f466fb to 0facfa2 Compare January 21, 2017 11:06
@@ -346,7 +346,7 @@ public IEnumerable<FileInfo> LoadInventory(FileInfo file)
file.Refresh();
if (!file.Exists)
{
throw new ArgumentException("Inventory file should exist.");

This comment was marked as spam.

This comment was marked as spam.

@TheRealPiotrP TheRealPiotrP force-pushed the piotrpMSFT/infra/reducetesttargetcomplexity branch from 49c27a2 to d6208d3 Compare January 22, 2017 21:58
@TheRealPiotrP
Copy link
Author

@MattGertz FYI, this infrastructure PR eliminates a bunch of the custom incremental build logic that CLI depended on while migrating CLI's build from PJ to CSProj. This greatly reduces CLI build complexity and, more importantly, build times. The PR also removes a deprecated and long-running test library [performance validation for project.json system which, after recent re-evaluation, adds little value for MSBuild/SDK scenarios. These will need to be replaced with propper performance tests for csproj in the near future].

The combined benefit was observed in this PR as a reduction from ~33 minute builds to ~18 minute builds. Wohoo!

@TheRealPiotrP TheRealPiotrP merged commit 1dfee9e into dotnet:rel/1.0.0 Jan 22, 2017
@TheRealPiotrP TheRealPiotrP deleted the piotrpMSFT/infra/reducetesttargetcomplexity branch January 22, 2017 22:40
@MattGertz
Copy link

:-)

@DustinCampbell
Copy link
Member

A lot of red in this PR, which is goodness in my book!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants